Skip to content

Avoid the per-request rotation copy in round-robin address selection#2230

Merged
hyperxpro merged 8 commits into
AsyncHttpClient:mainfrom
maygemdev:fix/round-robin-rotate-alloc
Jul 18, 2026
Merged

Avoid the per-request rotation copy in round-robin address selection#2230
hyperxpro merged 8 commits into
AsyncHttpClient:mainfrom
maygemdev:fix/round-robin-rotate-alloc

Conversation

@pavel-ptashyts

Copy link
Copy Markdown
Contributor

RoundRobinAddressSelector.rotateBy allocated a fresh ArrayList (backing array of size n) plus two subList views on every multi-IP round-robin request whose selected index != 0 — i.e. (n-1)/n of requests for an n-IP host — just to present the resolved addresses in round-robin failover order.

Return a lightweight read-only view (RotatedView, one small wrapper over the resolved list + a start index) instead of copying: element i maps to resolved.get((index + i) mod size). All consumers only read the result (get/size/iteration — NettyChannelConnector's failover loop and deprioritizeCooling), and the resolved list is not mutated after being wrapped, so no copy is needed. index == 0 and single-IP hosts still return the resolved list unchanged (no allocation), as before.

Only affects the opt-in LoadBalance.ROUND_ROBIN path. Existing selector tests (rotation order/evenness/failover completeness) pass unchanged; adds tests asserting the full element-by-element rotation order and that the returned view is read-only.

No API change: rotateBy and RotatedView are private; rotate()'s signature is unchanged.

RoundRobinAddressSelector.rotateBy allocated a fresh ArrayList (backing array of size n) plus two subList views on every multi-IP round-robin request whose selected index != 0 — i.e. (n-1)/n of requests for an n-IP host — just to present the resolved addresses in round-robin failover order.

Return a lightweight read-only view (RotatedView, one small wrapper over the resolved list + a start index) instead of copying: element i maps to resolved.get((index + i) mod size). All consumers only read the result (get/size/iteration — NettyChannelConnector's failover loop and deprioritizeCooling), and the resolved list is not mutated after being wrapped, so no copy is needed. index == 0 and single-IP hosts still return the resolved list unchanged (no allocation), as before.

Only affects the opt-in LoadBalance.ROUND_ROBIN path. Existing selector tests (rotation order/evenness/failover completeness) pass unchanged; adds tests asserting the full element-by-element rotation order and that the returned view is read-only.

No API change: rotateBy and RotatedView are private; rotate()'s signature is unchanged.
@hyperxpro

Copy link
Copy Markdown
Member

@pavel-ptashyts please fix merge conflicts

…e-alloc

# Conflicts:
#	client/src/main/java/org/asynchttpclient/netty/channel/RoundRobinAddressSelector.java
@pavel-ptashyts

Copy link
Copy Markdown
Contributor Author

@hyperxpro done

@hyperxpro
hyperxpro merged commit 2abf7ba into AsyncHttpClient:main Jul 18, 2026
1 check passed
@pavel-ptashyts
pavel-ptashyts deleted the fix/round-robin-rotate-alloc branch July 18, 2026 21:56
hyperxpro added a commit that referenced this pull request Jul 18, 2026
…eck (#2257)

Motivation:

#2230 replaced the per-request rotation copy in
`RoundRobinAddressSelector` with a lightweight read-only `RotatedView`.
Two follow-ups remained: the new view no longer implemented
`RandomAccess`, causing consumers that optimize for indexed access to
fall back to iterator-based traversal, and its bounds-checking behavior
was not covered by tests.

Modification:

Make `RotatedView` implement `RandomAccess` and add a
`rotatedViewRejectsOutOfBoundsAccess` test to verify that `get(size)`
and `get(-1)` throw `IndexOutOfBoundsException`.

Result:

`RotatedView` now preserves the `RandomAccess` contract of the
`ArrayList` it replaced, allowing consumers to retain indexed fast
paths, and its bounds-checking behavior is protected by regression
tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants